Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Operation beautify error messages #1521

Merged
merged 53 commits into from Oct 10, 2016

Conversation

felixmulder
Copy link
Contributor

@felixmulder felixmulder commented Sep 16, 2016

We want to provide beautiful and above all helpful error messages to the users of dotty. We aspire to do just as well as Elm and Rust in this category and this PR is the first step in that direction.

This PR adds a few things to error messages of dotty:

  • Colored output
  • Clearer distinction between individual error messages
  • Underlining of single line errors
  • Show line number on sourceline
  • Adds explanations via a -explain flag
  • Obeys the -color:option flag, never will give you a ConsoleReporter and the other options will give you the new FancyConsoleReporter
  • Adds error kinds (currently Syntax andType)
  • Adds string interpolator hl"1 + 2" for syntax highlighting
  • Obey pagewidth setting
  • Align error messages under error for better legibility
  • Add better structure to regular color highlighting instead of using Console directly
  • Make CI tests use boring reporter - so that tests may pass 👍
  • Make console reporters 1:1
    • Make FancyConsoleReporter and highlighting "color-aware"
    • Fix tests to obey new error messages
  • Improve explanation text highlighting
  • Improve syntax highlighter
  • Type diffs i.e. List[Map[A,C]] diff List[Map[B,C]]
    • Implement diffing on shown types
    • Refactor expanded2 and make sure that output from ex"$found" is equal to Mismatch(found)
      compiling:
  • Highlighting multiline errors
  • Insert error into correct point on multiline errors
object Test {
  try {
    2 + 3
  }
}

with ./bin/dotc -explain test.scala yields:

screen shot 2016-09-16 at 18 45 23

compiling:

object C1 { def foo: Int = 1 }
object C2 { def foo: Int = 2 }

object Test {
  import C1._
  import C2._

  println(s"The number you're looking for is: $foo")

  foo
}

yields:

screen shot 2016-09-16 at 18 48 06

@felixmulder felixmulder changed the title Better errormessages Better error messages Sep 16, 2016
@felixmulder felixmulder changed the title Better error messages Operation beautiful error messages Sep 16, 2016
@felixmulder felixmulder changed the title Operation beautiful error messages Operation beautify error messages Sep 16, 2016
@smarter smarter added the area:reporting Error reporting including formatting, implicit suggestions, etc label Sep 16, 2016
@felixmulder felixmulder force-pushed the topic/better-errormessages branch 2 times, most recently from f3db36d to d4b23e0 Compare September 18, 2016 07:08
@kiritsuku
Copy link

Did you consider IDE integration as a design goal? Right now, the IDE has to consume error messages as string, which of course is suboptimal because we have a rich GUI and therefore more abilities to render error messages. Especially things like color codes, which are made for the console are completely useless for IDE consumption. What we need is more a tree like structure that contains all of the semantic information (like error message, erroneous tree, position etc.) not as strings but as real semantic objects, which could then be transferred to an IDE. Maybe this is out of scope for this proposal (especially because there is no presentation compiler anyway) but I would still like to attract some attention to it.

@felixmulder
Copy link
Contributor Author

felixmulder commented Sep 18, 2016

@sschaef: both yes and no. The issue with IDE support is very open-ended and this could stretch the PR to absurdity. The current plan with this PR is to present an API for reporters that doesn't have to consume strings, but rather messages with semantic information (trees, symbols or whatever is relevant for the particular errors). But in the end they will be turned into strings and consumed by the console - at least for now.

We will provide this framework - and then we're hoping to get the community involved in making these error messages top notch! 🎉

Dotty is designed from the very beginning with a presentation compiler in mind. There just haven't been resources enough to get the ball moving - but once this PR is done, next big thing we will tackle is the presentation compiler.

Our current plan is to implement https://github.com/Microsoft/language-server-protocol

@kiritsuku
Copy link

Before you start to implement Microsofts protocol, I would like to have a chat with you (and the ensime people probably would want to know about this too), because you basically have to reimplement ensime and there are many things you can do wrong.

@felixmulder
Copy link
Contributor Author

We're always happy to collaborate - we'd love to have a word with both @fommil and you before we embark on this adventure. Sam has been very eager for us to start on this :)

When we get closer to starting the work on the presentation compiler, I'll ping you both - so, let me know your details on gitter and we'll set up a friendly chat.

@fommil
Copy link

fommil commented Sep 18, 2016

I am very keen on a presentation compiler, yes :-) But I wouldn't say my enthusiasm extends to the Microsoft protocol because i feel that should be provided outside of the compiler. E.g. by ensime or another impl. Also, the protocol demands much more than just the ast (e.g. source code resolving, classpath search, other languages, reverse lookup) and you would start to implement ensime from scratch which is fine but i think you'd be taking on more than you want :-D (and that would significantly restrict what could be achieved because it's then tied to compiler release schedules)

I'm on holiday for 3 weeks but if you are starting to spec out the presentation compiler please let's talk about ensime's requirements when we're both available. It could save you from implementing a lot of stuff, or missing something that would make our work easier! :-) // @rorygraves

@odersky
Copy link
Contributor

odersky commented Sep 18, 2016

Is there another reasonably stable API by which the presentation compiler
communicates with the IDE? I am asking because for Eclipse the lack of such
an API was our biggest problem. There were many misunderstandings what the
compiler was meant to provide and at what cost guarantees. The other
problem was that the API did not talk about possible race conditions. To me
the MS API looks attractive because it is quote detailed and seems to be
written by someone who understands cost tradeoffs. But if there is another,
simpler, lower-level API that does the job we should definitely consider it.

On Sun, Sep 18, 2016 at 8:22 PM, Sam Halliday notifications@github.com
wrote:

I am very keen on a presentation compiler, yes :-) But I wouldn't say my
enthusiasm extends to the Microsoft protocol because i feel that should be
provided outside of the compiler. E.g. by ensime or another impl. Also, the
protocol demands much more than just the ast (e.g. source code resolving,
classpath search, other languages, reverse lookup) and you would start to
implement ensime from scratch which is fine but i think you'd be taking on
more than you want :-D (and that would significantly restrict what could be
achieved because it's then tied to compiler release schedules)

I'm on holiday for 3 weeks but if you are starting to spec out the
presentation compiler please let's talk about ensime's requirements when
we're both available. It could save you from implementing a lot of stuff,
or missing something that would make our work easier! :-) // @rorygraves
https://github.com/rorygraves


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#1521 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwlVgWi4AtpgX1XVUj37MEthF3vXCr_ks5qrYFkgaJpZM4J_IU0
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@fommil in #1521: I am
very keen on a presentation compiler, yes :-) But I wouldn't say my
enthusiasm extends to the Microsoft protocol because i feel that should be
provided outside of the compiler. E.g. by ensime or another impl. Also, the
protocol demands much more than just the ast (e.g. source code resolving,
classpath search, other languages, reverse lookup) and you would start to
implement ensime from scratch which is fine but i think you'd be taking on
more than you want :-D (and that would significantly restrict what could be
achieved because it's then tied to compiler release schedules)\r\n\r\nI'm
on holiday for 3 weeks but if you are starting to spec out the presentation
compiler please let's talk about ensime's requirements when we're both
available. It could save you from implementing a lot of stuff, or missing
something that would make our work easier! :-) // @rorygraves"}],"action":{"name":"View
Pull Request","url":"https://github.com/lampepfl/dotty/
pull/1521#issuecomment-247864162"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

@kiritsuku
Copy link

The VSC protocol is by far the best protocol that is out there right now, indeed (and it is the one that has most chances to get most successful). However, it has a design goal one should understand. It wants to be a protocol for lightweight IDE functionality, it doesn't aim to provide a feature set of a full fledged IDE like Eclipse or IntelliJ.

Code completion or error highlighting are use cases the protocol wants to support. Because the tree of the compiler is not exposed as API, it is not possible that the consumer of the protocol implements its own functionality (like static analysis or refactorings). For scalac this has been different and Scala IDE and its feature set was made possible, even though exposing the trees came by a heavy price. By not exposing the trees, adopting the VSC protocol in the compiler is not attractive for Scala IDE - we simply wouldn't be able to support all of our rich features. The compiler could provide another API, just to make refactorings or static analysis possible but then the situation is mostly the same as in scalac.

Because of the above I started to push a heavy redesign of Scala IDE (see https://www.reddit.com/r/scala/comments/52w7n8/future_of_scala_ide_would_you_like_to_see_an/), whose goal it is to not consume any PC in the first place. But this requires further elaboration on the design restrictions, which I do not want to give here, since we are in the wrong thread. ;)

I see the VSC protocol in the compiler as a good middle ground for tools like the REPL/worksheet that want to provide some functionality of the compiler in an editor. But since ensime needs to be reimplemented in some parts in the compiler and since it is not realistic that Scala IDE is going to adopt the protocol, I'm not sure how attractive it is to spend time on implementing this in the first place.

@odersky
Copy link
Contributor

odersky commented Sep 19, 2016

On Mon, Sep 19, 2016 at 12:53 PM, Simon Schäfer notifications@github.com
wrote:

The VSC protocol is by far the best protocol that is out there right now,
indeed (and it is the one that has most chances to get most successful).
However, it has a design goal one should understand. It wants to be a
protocol for lightweight IDE functionality, it doesn't aim to provide a
feature set of a full fledged IDE like Eclipse or IntelliJ.

Code completion or error highlighting are use cases the protocol wants to
support. Because the tree of the compiler is not exposed as API, it is not
possible that the consumer of the protocol implements its own functionality
(like static analysis or refactorings). For scalac this has been different
and Scala IDE and its feature set was made possible, even though exposing
the trees came by a heavy price. By not exposing the trees, adopting the
VSC protocol in the compiler is not attractive for Scala IDE - we simply
wouldn't be able to support all of our rich features.

The compiler could provide another API, just to make refactorings or static
analysis possible but then the situation is mostly the same as in scalac.

So it's mostly refactorings that are missing from the VSC protocol? Which
static analyses do you have in mind which would be hard to do?

The problem with handing the tree out is that (a) trees are
compiler-specific, so hard to abstract over. (b) the typed trees that the
compiler produces are not very suitable anyway because they are heavily
desugared from source, and the desugarings cannot easily be reversed. And,
I see no reasonable way to change this. The essence of the Typer is that it
maps untyped trees to typed trees, but the typed trees have a different
structure from the untyped trees.

Cheers

  • Martin

Because of the above I started to push a heavy redesign of Scala IDE (see
https://www.reddit.com/r/scala/comments/52w7n8/future_
of_scala_ide_would_you_like_to_see_an/), whose goal it is to not consume
any PC in the first place. But this requires further elaboration on the
design restrictions, which I do not want to give here, since we are in the
wrong thread. ;)

I see the VSC protocol in the compiler as a good middle ground for tools
like the REPL/worksheet that want to provide some functionality of the
compiler in an editor. But since ensime needs to be reimplemented in some
parts in the compiler and since it is not realistic that Scala IDE is going
to adopt the protocol, I'm not sure how attractive it is to spend time on
implementing this in the first place.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1521 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAwlVrecNFswjrV30aTKcgJflsuGbRT3ks5qrmm2gaJpZM4J_IU0
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@sschaef in #1521: The
VSC protocol is by far the best protocol that is out there right now,
indeed (and it is the one that has most chances to get most successful).
However, it has a design goal one should understand. It wants to be a
protocol for lightweight IDE functionality, it doesn't aim to provide a
feature set of a full fledged IDE like Eclipse or IntelliJ.\r\n\r\nCode
completion or error highlighting are use cases the protocol wants to
support. Because the tree of the compiler is not exposed as API, it is not
possible that the consumer of the protocol implements its own functionality
(like static analysis or refactorings). For scalac this has been different
and Scala IDE and its feature set was made possible, even though exposing
the trees came by a heavy price. By not exposing the trees, adopting the
VSC protocol in the compiler is not attractive for Scala IDE - we simply
wouldn't be able to support all of our rich features. The compiler could
provide another API, just to make refactorings or static analysis possible
but then the situation is mostly the same as in scalac.\r\n\r\nBecause of
the above I started to push a heavy redesign of Scala IDE (see
https://www.reddit.com/r/scala/comments/52w7n8/future_
of_scala_ide_would_you_like_to_see_an/), whose goal it is to not consume
any PC in the first place. But this requires further elaboration on the
design restrictions, which I do not want to give here, since we are in the
wrong thread. ;)\r\n\r\nI see the VSC protocol in the compiler as a good
middle ground for tools like the REPL/worksheet that want to provide some
functionality of the compiler in an editor. But since ensime needs to be
reimplemented in some parts in the compiler and since it is not realistic
that Scala IDE is going to adopt the protocol, I'm not sure how attractive
it is to spend time on implementing this in the first
place."}],"action":{"name":"View Pull Request","url":"https://
github.com//pull/1521#issuecomment-247963154"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

@cvogt
Copy link
Member

cvogt commented Sep 19, 2016

Making error message rendering pluggable would be great and allow tool specific rendering and outsourcing rendering to non-compiler-hackers. An intermediate storage format (json or else) is one way but has problems as @odersky pointed out. An alternative idea @tpolecat and I were discussing at Scala World would be providing rendering callbacks to the compiler. Similar to macros in a way as they would be run by the compiler, but going from the data of the situation to String, e.g. (found: Type, required: Type) => String. Tools would need to be able to provide their own callbacks.

@kiritsuku
Copy link

Discussion about the PC is continued in https://github.com/lampepfl/dotty/issues/1523

@kiritsuku
Copy link

Not sure if the ^ are the expected output:

scala> val x = 0
x: Int = 0
scala> x = List(List(0))
-- Error: <console> ----------------------------------------------------------------------------------------------------
4:x = List(List(0))
          ^^^^^^^^^^^^^
      type mismatch:
      found:    scala.collection.immutable.List[scala.collection.immutable.List[Int]]
      required: Int
-- Error: <console> ----------------------------------------------------------------------------------------------------
4:x = List(List(0))
    ^^^^^^^^^^^^^^^^^
  reassignment to val

It continues outside of the character range that is typed.

@kiritsuku
Copy link

kiritsuku commented Sep 19, 2016

This shows the same error multiple times:

scala> try 1
-- [E002] Syntax Warning: <console> ------------------------------------------------------------------------------------
0:try 1
      ^
      A try without catch or finally is equivalent to putting
      its body in a block; no exceptions are handled.
-- [E002] Syntax Warning: <console> ------------------------------------------------------------------------------------
1:try 1
      ^
      A try without catch or finally is equivalent to putting
      its body in a block; no exceptions are handled.
-- [E002] Syntax Warning: <console> ------------------------------------------------------------------------------------
1:try 1
      ^
      A try without catch or finally is equivalent to putting
      its body in a block; no exceptions are handled.
-- [E002] Syntax Warning: <console> ------------------------------------------------------------------------------------
5:}
  ^
  A try without catch or finally is equivalent to putting
  its body in a block; no exceptions are handled.
res4: Int = 1

@kiritsuku
Copy link

I do not like that the error message no longer starts in column 0. I don't see a benefit in doing this.

@kiritsuku
Copy link

The dividing line that also contains the error message doesn't fill the size of the terminal. It looks like the size is hardcoded.
line

@felixmulder
Copy link
Contributor Author

felixmulder commented Sep 20, 2016

@sschaef:

Not sure if the ^ are the expected output:

  • Positions for these cases are odd - I'll have a look at it.

This shows the same error multiple times:

  • REPL issue - not specific to this PR - still, I'll have a look.

The dividing line that also contains the error message doesn't fill the size of the terminal. It looks like the size is hardcoded.

There's the flag ./bin/dotr -pagewidth X to set the width - same as being used for other prettyprinting.

if [ -z "$1" ]; then
echo "Starting dotty REPL..."
eval "$DOTTY_ROOT/bin/dotc -repl"
elif [[ ${first_arg:0:1} == "-" ]]; then
eval "$DOTTY_ROOT/bin/dotc -repl $@"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sophisticated enough for now - we will have to revisit this script to get it on par with the scala script eventually - see: #1526

@fommil
Copy link

fommil commented Sep 22, 2016

In terms of integration with straight emacs etc (no ensime) it is really important that the file name, line number and column number (from 0) is available in an easily parseable form, typically colon delimited with some kind of identifier string like WARN or ERROR. For the pc, anything goes.

@felixmulder
Copy link
Contributor Author

@fommil: thank you for bringing attention to this. I assume it is the scala-mode2 being maintained as an (independent) part of Ensime?

Currently the default reporter on master does not give a column at all - so this would need to be remedied anyhow for this to work. I'm curious as to what the functionality that depends on this is - and if said functionality can be improved.

@felixmulder felixmulder force-pushed the topic/better-errormessages branch 3 times, most recently from 75f2e3e to 949c974 Compare September 22, 2016 17:58
Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@smarter Indeed it was modeled after how scalac shows ConstantTypes. I think in the example you gave it's clearer than the alternatives.

ex"""reference to $name is ambiguous;
|it is both ${bindingString(newPrec, ctx, "")}
|and ${bindingString(prevPrec, prevCtx, " subsequently")}""",
ex"""|reference to `$name` is ambiguous
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the reason for the first | after the """?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we're applying stripMargin to the result of the string interpolation, the result is the same with or without the first |.

I simply like it present since it can then be aligned with the other pipe symbols showing clearly what the resulting string will look like.

@odersky
Copy link
Contributor

odersky commented Oct 10, 2016

OK. I think it aligns just fine without the first | but whatever :-)

On Mon, Oct 10, 2016 at 8:11 PM, Felix Mulder notifications@github.com
wrote:

@felixmulder commented on this pull request.

In src/dotty/tools/dotc/typer/Typer.scala
#1521:

@@ -166,9 +171,9 @@ class Typer extends Namer with TypeAssigner with Applications with Implicits wit
else {
if (!scala2pkg && !previous.isError && !found.isError) {
error(

  •          ex"""reference to $name is ambiguous;
    
  •              |it is both ${bindingString(newPrec, ctx, "")}
    
  •              |and ${bindingString(prevPrec, prevCtx, " subsequently")}""",
    
  •          ex"""|reference to `$name` is ambiguous
    

Since we're applying stripMargin to the result of the string
interpolation, the result is the same with or without the first |.

I simply like it present sine it can then be aligned with the other pipe
symbols showing clearly what the resulting string will look like.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#1521, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAwlVkbaz-ycoJYBXdxmEeu0MqG7UoMbks5qyn_RgaJpZM4J_IU0
.
{"api_version":"1.0","publisher":{"api_key":"
05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":
{"external_key":"github/lampepfl/dotty","title":"
lampepfl/dotty","subtitle":"GitHub repository","main_image_url":"
https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-
11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://
cloud.githubusercontent.com/assets/143418/15842166/
7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in
GitHub","url":"https://github.com/lampepfl/dotty"}},"
updates":{"snippets":[{"icon":"PERSON","message":"@felixmulder commented
on #1521"}],"action":{"name":"View Pull Request","url":"https://
github.com//pull/1521"}}}

Prof. Martin Odersky
LAMP/IC, EPFL

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:reporting Error reporting including formatting, implicit suggestions, etc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants